Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Archetype TypeInfo::type_name accessor #980

Merged
merged 1 commit into from
Dec 2, 2020
Merged

Add Archetype TypeInfo::type_name accessor #980

merged 1 commit into from
Dec 2, 2020

Conversation

smokku
Copy link
Member

@smokku smokku commented Dec 2, 2020

This adds an accessor to get the type name from Archetype.

I use this in ECS debugger when the Reflect is not available, to at least show the name of the Component type.

@cart cart merged commit 4833c2a into bevyengine:master Dec 2, 2020
@cart
Copy link
Member

cart commented Dec 2, 2020

Hmm this breaks release builds for some reason (both on stable and nightly). I need to revert this. Can you reproduce this error on your machine?

image

cart added a commit that referenced this pull request Dec 2, 2020
cart added a commit that referenced this pull request Dec 2, 2020
@cart
Copy link
Member

cart commented Dec 2, 2020

This feels like a compiler bug. Debug vs release builds shouldn't handle this case differently. I also have no clue how to resolve this / disambiguate 😄

@smokku
Copy link
Member Author

smokku commented Dec 2, 2020

Same here. Works in --debug does not compile in --release.

I see two solutions. Rename the getter to .get_type_name() (I don't like get_ getters)
or just make the field pub (smells iffy too).

@cart
Copy link
Member

cart commented Dec 2, 2020

I'm just so confused. We use this pattern everywhere.

@smokku
Copy link
Member Author

smokku commented Dec 2, 2020

With a bit of help from rust-lang Discord I present you #985

Can you spot the difference? 😜

@cart
Copy link
Member

cart commented Dec 2, 2020

Haha it took me a second. You left the debug_assertions in this time? Why do we need that?

@smokku
Copy link
Member Author

smokku commented Dec 2, 2020

No, I removed the cfg this time. It caused the field to be missing in release builds.
With the field missing, the code that wants to access it does not compile. :-)

@cart
Copy link
Member

cart commented Dec 2, 2020

Lol apparently i had my tabs mixed up. Glad to hear the world still makes sense.

@fopsdev fopsdev mentioned this pull request Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants